Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stall-analyser: use 0 for default value of --minimum #2306

Merged

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Jun 27, 2024

this change is a cleanup. it:

  • uses a better name for the --minimum option, args.minimum
    is way too general.
  • uses the default value of 0 instead of None, simpler this way
  • just references arg.tmin, simpler this way

@bhalevy
Copy link
Member

bhalevy commented Jun 27, 2024

Hi @tchaikov, sorry, I think I've beat you to the gong.
Please see #2305

@xemul
Copy link
Contributor

xemul commented Jun 28, 2024

Presumably obsoleted by #2305 ?

this change is a cleanup. it:

- use a better name for the `--minimum` option, `args.minimum`
  is way too general.
- use the default value of 0 instead of `None`, simpler this way
- just reference `arg.tmin`, simpler this way

Signed-off-by: Kefu Chai <[email protected]>
@tchaikov tchaikov force-pushed the stall-analyser-handle-empty-file branch from f8f5875 to 5fa4c9c Compare June 28, 2024 15:59
@tchaikov tchaikov changed the title stall-analyser: avoid referencing undefined variable stall-analyser: use 0 for default value of --minimum Jun 28, 2024
@tchaikov
Copy link
Contributor Author

Presumably obsoleted by #2305 ?

i degraded it from a fix to a cleanup .

Copy link
Member

@bhalevy bhalevy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@bhalevy
Copy link
Member

bhalevy commented Jul 1, 2024

@scylladb/seastar-maint please consider merging

@avikivity avikivity merged commit 03942b2 into scylladb:master Jul 1, 2024
14 checks passed
@tchaikov tchaikov deleted the stall-analyser-handle-empty-file branch July 1, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants